media: Add InitializeWithUrl transport and URL player renderer - #10989
media: Add InitializeWithUrl transport and URL player renderer#10989abhijeetk wants to merge 2 commits into
Conversation
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
ac56964 to
30d4223
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces UrlPlayerRenderer and its Mojo wrapper UrlPlayerRendererWrapper to support URL-based media playback on tvOS using Starboard. The code review feedback focuses on improving robustness, safety, and style guide compliance. Key recommendations include replacing unsafe CHECK and DCHECK statements with graceful runtime checks, initializing primitive local variables to prevent undefined behavior, adding defensive null checks for client_ and player_bridge_ to avoid crashes, and removing unused private member variables. Additionally, the reviewer notes that both new classes require detailed class comments explaining their lifetime, ownership, and threading models to adhere to the repository style guide.
|
Since url player is only on tvOS, just want to check if checking is_tvos is sufficient? |
No,
|
Add use_starboard_url_player GN capability flag, derived from use_starboard_media and tvOS platform detection. Expose it as BUILDFLAG(USE_STARBOARD_URL_PLAYER) through build_config.h and register matching Mojo enabled_features entry. Add RendererType::kUrlPlayer = 13 to the C++ enum, Mojom enum, traits mapping, and histogram. The new enum is gated behind USE_STARBOARD_URL_PLAYER so non-tvOS builds are unaffected. This establishes the capability boundary for all subsequent URL-player CLs. Bug: 512045535
|
cc : @jkim-julie @Gyuyoung @rakuco |
Introduce InitializeWithUrl(client, source_url) on the Mojo renderer pipe to carry the media URL from the renderer process to the GPU process, keeping URL delivery and initialization ordered on the same pipe. On the GPU side, add UrlPlayerRenderer for tvOS URL-based playback via SbPlayerBridge in punch-out mode, and UrlPlayerRendererWrapper as the Mojo service bridge. The wrapper sets the URL on the renderer then calls Initialize(). On the renderer-process side, MojoRenderer and MojoRendererWrapper forward InitializeWithUrl over the Mojo pipe. MojoRendererService validates the renderer type and URL before forwarding, with runtime guards for unexpected state and invalid URLs. Bug: 512045535
In this case, I think the gating logic should be like this: Starboard should contain multiple platforms (android, tvOS, and 3P devices), so the flag |
|
Thanks for all review comments. We are moving ahead with approach in PR #10698 (UrlPlayer using StarboardRenderer). |
Add mojom::Renderer::InitializeWithUrl(client, source_url) to deliver
the URL on the renderer Mojo pipe, keeping URL delivery and
initialization ordered on the same pipe.
Add UrlPlayerRenderer for tvOS URL-based playback via SbPlayerBridge
and UrlPlayerRendererWrapper as the Mojo service bridge. The wrapper's
InitializeWithUrl() sets the URL on the renderer internally then calls
Initialize().
MojoRendererService validates the renderer type and URL before
forwarding via static_cast, guarded by GetRendererType() == kUrlPlayer
and ReportBadMessage().
Bug: 512045535